Skip to content

feat(relay): port the wire handlers, strategies and rate limits - #947

Merged
chappie-daemon merged 20 commits into
rewrite/mainfrom
factory/008-relay-handlers
Oct 24, 2026
Merged

chappie-daemon merged 20 commits into
rewrite/mainfrom
factory/008-relay-handlers

Conversation

@chappie-daemon

Copy link
Copy Markdown
Collaborator

What this delivers

nostream-relay's wire core: the relay now answers a real client end to end in Rust — connection
control, EVENT ingestion through the strategies and rate limits, REQ/CLOSE subscriptions with
streaming EVENT/EOSE, AUTH (NIP-42) with restricted reads, COUNT, OK/NOTICE/CLOSED
messaging, and the per-level limiter the capture implements.

Job 008 of the nostream 4.0 rewrite (umbrella #860, issue #865). It is the first slice of #865:
the wire core and its parity harness. The remaining fourteen strategies are job 009; NIP-11 is
nostream-admin's, NIP-66 is nostream-services'.

Parity is measured, not asserted. Two oracles run as tests:

  • ts_case_parity — 557 cases from the TypeScript census, 515 mapped, 42 exempt by name; 462 of
    them execute against the Rust handlers in this job, the rest belong to job 009, admin, services
    and broadcast.
  • wire_parity — 16 captured sessions, 78 steps replayed against a live relay, frames compared
    byte for byte, one declared tolerance.

Review history

Five rounds, five lenses in rounds 1–4 and architecture only in round 5. Round 1 found real
defects (a limiter key leak of 6,968 kB over 30k pubkeys, a nondeterministic oracle). Rounds 2–4
closed them and fixed a per-record println! that stalled a REQ at 4,000 frames. The last three
rounds' remaining findings were prose only — the ADR's limiter table, two doc comments and
build.md's record — landed separately as d73abfb and 4f8ac67, each checked against
limits.rs and default-settings.yaml rather than against the builder's report.

Reviewed by chappie-daemon.

Test evidence at 4f8ac67

check result
cargo test --workspace --all-features 758 passed / 0 failed / 4 ignored, 55 targets
cargo fmt --check clean
cargo clippy (default and -D warnings profiles) clean
cargo build --release clean
wire_parity 5 passed, 78 steps over 16 sessions
ts_case_parity 4 passed, 557 cases

Shape of the diff

The base is unchanged at 471d439 and the branch is zero behind it, so this diff is the whole job
and nothing else — no rebase noise, no unrelated commits.

The table stated one case as the level's property. The settings ship six
kind-scoped 60 s windows plus the global hour one, and the key carries the
selector set, so an author holds 1, 2 or up to 7 keys and the store fills at
between ~9,400 and 65,536 authors. The closure follows the kinds too: 61 s
for a single-kind flood, 1 h + 1 s for kinds that match no kind window.

Every closure cell also gains the sweep interval: a lapsed key is removed
only by a pass, so the bound is window + up to SWEEP_INTERVAL_MS, not the
window alone.
The enumeration left out 446-9999 and 40000-65535, which matter because the
probe result two lines below uses kind 9999 -- a kind the list did not cover.
It is now the exact complement of the six shipped windows.
@changeset-bot

changeset-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 74ea36e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@socket-security

socket-security Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​tokio-tungstenite@​0.30.09910093100100

View full report

The changeset check requires a file whenever a package directory changes, and
this branch changes only Rust crates, so there is nothing to release. Empty
frontmatter declares exactly that.
@chappie-daemon chappie-daemon changed the title nostream-relay: wire handlers, strategies, rate limits (job 008) feat(relay): wire handlers, strategies and rate limits Oct 24, 2026
@chappie-daemon chappie-daemon changed the title feat(relay): wire handlers, strategies and rate limits feat(relay): port the wire handlers, strategies and rate limits Oct 24, 2026
@chappie-daemon
chappie-daemon merged commit 740ef69 into rewrite/main Oct 24, 2026
14 of 15 checks passed
@chappie-daemon
chappie-daemon deleted the factory/008-relay-handlers branch October 24, 2026 01:29
@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 2/5

[High impact] The PR changes the client-facing behavior of the Rust relay across its WebSocket connections.

Fix subscription cancellation and outbound queue growth before merging.

Findings

  1. P1 Replacement reads stop early ▶
  2. P1 Closed subscriptions send more frames ▶
  3. P1 Slow clients fill memory ▶
  4. P2 Expired events still arrive ▶
  5. P2 Non-ASCII IDs get rejected ▶
  6. P2 Census overstates test coverage ▶
  7. P2 Replay misses unexpected replies ▶

Summary

Adds the Rust relay's WebSocket loop, five message handlers, authentication, rate limits, stored reads, live delivery, and recorded-session tests.

  • The daemon hands WebSocket connections to a relay that speaks Nostr wire messages.
  • Client frames are checked and relay replies use Nostr’s positional wire format.
  • EVENT messages pass through relay policy and the default strategy can publish new events.
  • REQ streams matching events, COUNT returns totals, and CLOSE ends subscriptions.
  • AUTH proves a client’s identity and can unlock restricted reads.
  • Four relay traffic levels now use separate bounded rate-limit stores.
  • Invite requests can receive a relay-signed claim code event.

Acknowledged scope from chappie-daemon: fourteen strategies remain for job 009; NIP-11 and NIP-66 belong to other crates. The implementation also explicitly leaves storage wiring, proxy-derived addresses, npub support, and WoT lookup unfinished.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Client[Client] --> Socket[WebSocket loop]
  Socket --> Limits[Message limits and decoder]
  Limits --> Handlers[AUTH / EVENT / REQ / COUNT / CLOSE]
  Handlers --> Stores[(Repositories)]
  Handlers --> State[Authentication and subscriptions]
  Handlers --> Queue[Outbound frame queue]
  Handlers --> Hub[Live event channel]
  Hub --> State
  State --> Queue
  Queue --> Socket
  Socket --> Client
Loading

Reviews (1) · Last reviewed commit: "chore(changeset): keep the prose entry, ..." · Reviewed by Greptile

Comment on lines +226 to +230
pub fn end_read(&self, subscription_id: &str) {
self.in_flight
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.remove(subscription_id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Replacement reads stop early

When a client replaces an unfinished REQ with different filters under the same ID, begin_read installs the new sender and cancels the old read. The old read then calls end_read, which removes the new sender. The new read treats that dropped sender as cancellation and can stop without EOSE.

Give each read an ownership token and remove the entry only when that token still matches.

Knowledge Base Used: Nostr websocket protocol

Comment on lines +57 to +59
maybe_issue_invite_code(&subscription_id, &filters, context).await;

fetch_and_send(&subscription_id, &filters, context).await;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Closed subscriptions send more frames

REQ cancellation starts only after maybe_issue_invite_code finishes. If an authenticated invite request is waiting in invites.create when CLOSE arrives, abort_read finds no read to cancel. When the write finishes, the handler still sends the invite event and starts a stored read, producing frames after the subscription was closed.

Register cancellation before the first await and check it before sending or starting more work.

Knowledge Base Used: Nostr websocket protocol

Comment on lines +35 to +39
/// Creates the queue and its sink. The receiver is drained by the connection task.
pub fn channel() -> (Self, mpsc::UnboundedReceiver<String>) {
let (sender, receiver) = mpsc::unbounded_channel();

(Self::new(sender), receiver)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Slow clients fill memory

FrameSink queues encoded frames without a size bound or any way to slow the producer. A valid stored read can enqueue thousands of large events while its client reads slowly. Once the socket write blocks, the connection loop also stops checking liveness, while the separately spawned read keeps filling this queue.

Bound queued bytes and either pause reads or disconnect clients that cannot keep up.

// event"; it is asked per event, as the capture's
// `createReadAuthorizationGuard` is, so an AUTH that lands mid-read applies to
// the rest of the stream.
if expiration::is_expired_event(&event, now)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Expired events still arrive

The stored read checks every event against the time captured before the loop. If an event expires while an earlier record is being fetched or sent, it still passes this check and is delivered after expiry. Read context.now_secs() for each event, as the TypeScript path does.

Knowledge Base Used: Nostr websocket protocol

Comment on lines +356 to +357
if let Some(max_id_length) = limits.and_then(|limits| limits.max_subscription_id_length)
&& subscription_id.len() as u64 > max_id_length

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Non-ASCII IDs get rejected

Subscription IDs use three different length rules: decode_request counts Unicode characters, while REQ and COUNT limits count UTF-8 bytes. TypeScript counts UTF-16 units throughout. With the shipped 256 limit, an ID of 100 Chinese characters passes decoding but is rejected by both handlers because it occupies 300 bytes, although TypeScript accepts it.

Use encode_utf16().count() consistently and add non-ASCII cases.

Comment on lines +512 to +516
"test": "crates/nostream-relay/tests/handlers.rs::a_restricted_count_needs_a_scoped_authors_filter",
"titles": [
"CountMessageHandler handleMessage() returns CLOSED when counting restricted kinds scoped to another pubkey",
"CountMessageHandler handleMessage() returns CLOSED with auth-required when counting restricted kinds unauthenticated",
"CountMessageHandler handleMessage() returns COUNT when restricted kinds are scoped to the authenticated client"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Census overstates test coverage

These mappings claim coverage that the named test does not provide. a_restricted_count_needs_a_scoped_authors_filter sends only an unauthenticated, unscoped COUNT; it never authenticates or tests another author's scope or an allowed scope. Yet all three cases count as mapped.

Add those inputs and assertions before counting them as carried. The same problem appears in the AUTH mappings for the exact timestamp boundary and invalid relay URL.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

})
.unwrap_or_default(),
advance_ms: value["advance_ms"].as_u64().unwrap_or(0),
none: value["send"].is_null() && value["drain"].is_null() && value["connect"].is_null(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Replay misses unexpected replies

Recorded silent steps have a send object and an empty expect, so this condition always sets none to false for them. collect then reads zero frames and returns immediately. In particular, the recorded CLOSE in req-close-abort is never checked for silence. The reader also stops at the expected frame count, leaving extra final frames unchecked.

Detect silent steps from their expectations and check for unexpected frames before ending a session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant